Open
Conversation
간편한 통합테스트를 위한 - run() 메서드 분리 - int 반환값
validateConsecutiveOperators() -> validateOperatorPosition() MESSAGE_ERROR_CONSECUTIVE_OPERATORS -> MESSAGE_ERROR_WRONG_POSITION_OPERATORS 연속적인 연산자인 경우 뿐만 아니라 연산자가 처음 입력되는 경우에도 같은 분기로 처리되므로 적절한 이름으로 변경하였습니다.
UiHyeon-Kim
reviewed
Apr 6, 2025
Member
UiHyeon-Kim
left a comment
UiHyeon-Kim
left a comment
There was a problem hiding this comment.
코드 잘 보고 갑니다. 자바 부분은 실력이 부족해 큰 도움이 못 되어 드린 것 같네요.
1주차 고생 많으셨습니다~ 제 욕심이지만 다음 과제부터는 패키지 분리 및 아키텍처 적용을 해보면 현제님께 조금 더 도움이 될 것 같아요!
|
|
||
| console.printResult(result); | ||
| return result; | ||
| } |
Member
There was a problem hiding this comment.
Application 클래스가 프로그램의 진입점을 담당하는데, run 까지 넣으면 책임이 조금 과중된다 생각합니다!
ApplicationTest를 통합 테스트로 구현하고 싶어 하신 것 같은데, run 또한 객체이기 때문에 따로 분리하고 main 함수 호출을 통해 테스트를 진행하는 것이 조금 더 좋은 방향이라고 생각해요.
또한, run이 static으로 선언되었고, 다른 클래스를 의존성을 주입받는 것이 아니라 내부에 선언함으로써 유지보수 및 테스트가 어려워진다고 알고있어요.
예를 들어, 현재 calculator가 아닌 연산자 우선순위를 고려한 클래스를 따로 만든다면, 이를 의존성만 바꾸면 되는 것이 아닌 내부 코드를 바꿔야 합니다.
조금 길어졌지만 OOP의 SRP와 OCP에 대해 조금 찾아보시면 현제님이라면 금방 익숙해지실 것 같아요!
| public static final String MESSAGE_ERROR_EMPTY_OR_BLANK = "[ERROR] 입력값이 없거나 공백입니다. 입력 형식을 확인해 주세요."; | ||
|
|
||
|
|
||
| private static final Scanner scanner = new Scanner(System.in); |
Member
There was a problem hiding this comment.
Console을 따로 만들어서 입출력을 담당해 책임 분리가 좋은 것 같아요.
Scanner를 싱글톤으로 만들면 어떤 이점이 있나요?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
연산 시
Queue가 번뜩여서 큐로 처리해봤는데찾아보니 성능이 더 좋다고는 하는데 어떤지 모르겠네요..
ArrayList를 사용하는 게 맞을까요? 입력값을 숫자와 연산자 기준으로 나눠서 배열로 저장하려고 썼는데테스트코드 작성할 때나 인스턴스 메서드 쓰는데 조금 이질감이 있네용..
단위 테스트는 메서드 분리하면서 쉽게 진행했는데 통합테스트는 main에서 어떻게 분리해야될 지 모르겠네요.
일단 분리해서 통합테스트코드도 작성했는데 어떨까요? 편해서 만족스럽긴 합니다 😂
늦게 제출해서 죄송합니다 😥😥😥
게을러서 늦게 시작했더니 생각보다 엄청 오래걸리네요.. 😭
회고 🙏🙏🙏?